Add fallback when both co- and contra-variant inference candidates exist#54072
Add fallback when both co- and contra-variant inference candidates exist#54072ahejlsberg merged 3 commits intomainfrom
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at f0f477e. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at f0f477e. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at f0f477e. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at f0f477e. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at f0f477e. You can monitor the build here. |
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @ahejlsberg, the results of running the DT tests are ready. |
|
@typescript-bot pack this |
|
Heya @jakebailey, I've started to run the tarball bundle task on this PR at f0f477e. You can monitor the build here. |
|
Hey @jakebailey, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
src/compiler/checker.ts
Outdated
| every(context.inferences, other => other === inference || | ||
| getConstraintOfTypeParameter(other.typeParameter) !== inference.typeParameter || |
There was a problem hiding this comment.
Part of #52180 was motivated by noticing that when we treat the "current" inference as special, we incur some order-dependent behavior. Where this came up (if my memory was correct) was when I was editing the strictFunctionTypes PR, where I could edit the file and see errors change around non-deterministically.
I tested restoring the change from #52180 and this PR still passes (i.e. putting back other !== inference && getConstraintOfTypeParameter(other.typeParameter) !== inference.typeParameter), which makes me feel like reverting it back to the state in #52123 that skips over other === inference will bring back that oddity (and we just don't have a test for it).
I'll try and see if I can make it break again like I did the first time.
There was a problem hiding this comment.
It turns out we deleted the code where I was able to reproduce this, so, I guess I don't really know anymore.
There was a problem hiding this comment.
Oh! You're right, I missed that new line.
There was a problem hiding this comment.
In that case, I'm happy with whichever is more performant or clearer. They're definitely equivalent now that I think closely.
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at c377c44. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at c377c44. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at c377c44. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at c377c44. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at c377c44. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
@ahejlsberg Here they are:Comparison Report - main..54072
System
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @ahejlsberg, the results of running the DT tests are ready. |
3 participants
In cases where both co- and contra-variant inference candidates exist, this PR adds the ability to fall back to the secondary inference when the constraint check fails for the primary inference.
Fixes #54005.